Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manifest,images: add new manifest.NewBuildFromContainerSpec and use in new BootcDiskImage (HMS-3318) #354

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Jan 8, 2024

This branch prepares for allowing to have build roots from container sources. This is a bit of a shortcut PR that is mainly tailored to bootc-image-builder.

Alternatively we could change InstantiateManifest() to not take repos []rpmmd.RepoConfig but instead something like:

type RepoContainerStuff struct {
    Repos []rpmmd.RepoConfig
    Containers []container.SourceSpec
}

type ImageKind interface {
	Name() string
	InstantiateManifest(m *manifest.Manifest, repoContainer *RepoContainerStuff, runner runner.Runner, rng *rand.Rand) (*artifact.Artifact, error)
}

of course with a more sensible name. Happy to do this if that is the preferred way. I will move this out of draft (or close it) once I get feedback on this (or maybe there is a third option).

Once this is available, bib will use:

img := image.NewBootcDiskImage(containerSource)
...
_, err = img.InstantiateManifestFromContainers(&mf, containerSources, runner, rng)

Note that this is not sufficient for a full container based buildroot, the following is missing:

  1. get EFI binaries from an alternative location (e.g. via stages(grub2): allow pulling efi binaries from alternative efi roots osbuild#1529 or bootupd)
  2. create a second "bib-build" pipeline that uses the "bib" container as the buildroot and run "qemu-img" from there

@mvo5 mvo5 requested a review from achilleas-k January 8, 2024 17:15
@mvo5 mvo5 changed the title manifest,images: add new manifest.NewBuildFromContainerSpec and use in new BootcDiskImage manifest,images: add new manifest.NewBuildFromContainerSpec and use in new BootcDiskImage (HMS-3318) Jan 9, 2024
Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we discussed this elsewhere but I would prefer if instead we created a Build pipeline interface with two implementations: an RPM-based one, which is the current Build pipeline, and a new container-based one.

@achilleas-k achilleas-k self-requested a review January 9, 2024 20:05
This commit allows to create a new build root from a container
spec.
To support build roots based on container inputs we need a way to
specify those with images. This commit provides a way to do this.
Add a new `BootcDiskImage` type that is more opinionated then
OSTreeDiskImage as the two are more and more diverging.

This will be used in `bootc-image-builder`.
@mvo5
Copy link
Contributor Author

mvo5 commented Jan 10, 2024

I think we discussed this elsewhere but I would prefer if instead we created a Build pipeline interface with two implementations: an RPM-based one, which is the current Build pipeline, and a new container-based one.

Thank you! I forgot, fixed this now. Please let me know if that matches what you had in mind :)

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I like this :)

Any reason it's still draft? Should we add an image build of some sort for testing?

@mvo5 mvo5 marked this pull request as ready for review January 10, 2024 18:55
@mvo5
Copy link
Contributor Author

mvo5 commented Jan 10, 2024

Nice! I like this :)

Any reason it's still draft? Should we add an image build of some sort for testing?

Mostly draft as I was not 100% certain if I got the idea right :) Un-drafted now. I am happy to add more testing, I will look into how to do this best, it's slightly incomplete still (but should be enough to build raw images already with bib).

@achilleas-k achilleas-k added this pull request to the merge queue Jan 10, 2024
@achilleas-k
Copy link
Member

Let's merge it then and see what happens :D

Merged via the queue into osbuild:main with commit 733ec74 Jan 10, 2024
10 checks passed
@mvo5 mvo5 deleted the builtroot-from-container branch January 11, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants